WIP [16.0] add account_ecotax module #428

mourad-ehm · 2024-05-23T15:28:39Z

No description provided.

account_ecotax/__manifest__.py

account_ecotax/models/account_ecotax_classification.py

account_ecotax/models/account_move_line.py

yankinmax

LGTM: some nitpicking + code review (not an expert in France taxes)

account_ecotax/models/account_move_line.py

account_ecotax/tests/test_ecotax.py

bealdav · 2024-06-19T08:16:21Z

account_ecotax/models/account_tax.py

+        '1: check  "included in base amount "\n'
+        "2: The Ecotax sequence must be less then "
+        "VAT tax (in sale and purchase)",
+    )


Maybe we may rename is_ecotax to ecotax as fields Selection (('fixed', 'Fixed'), ('weight', 'Weight')] because it can lead to misconfiguration.

Domain can prevent this error.

cc @mourad-ehm @yankinmax ?

Also there case where ecotax should be considered as surface or volume, not implemented there but could be with additional entry in field Selection

is_ecotax and ecotax_type are different fields, so no misconfiguration here.
Taxes are filtered by is_ecotax in computations.
I don't see issue here actually. But, I can miss something of course

You're they are different fields but they might the same with a selection field in a mixin class.

Here ecotax computing fails because product _compute_product_ecotax method search for a fixed ecotax amount and sale order apply 'ecotaxe poids' , then ecotax is 0 on sale order line because there is only fixed ecotax amount not computed

here is code for weight tax case,

and commented line for fixed case is below

yankinmax · 2024-06-19T08:22:42Z

account_ecotax/tests/test_ecotax.py

+        Ecotax classification data for this test:
+            - fixed type
+            - default amount: 5.0
+        Product data for this test:
+            - list price: 100
+            - fixed ecotax
+            - no manual amount
+
+        Expected results (with 1 line and qty = 1):
+            - invoice ecotax amount: 5.0
+            - invoice total amount: 100.0
+            - line ecotax unit amount: 5.0
+            - line ecotax total amount: 5.0
+        """


why invoice total amount 100 and not 120?

It seems to me you're right. @mourad-ehm will have a look tomorrow

yankinmax · 2024-06-19T08:24:47Z

BTW: I'm on migration of account_ecotax to v17:

[17.0][MIG] account_ecotax: Migration to 17.0 #433
There are some issues in the tests here. Check v17 variant _init_invoice method pls.

And the most important account.move::_get_tax_totals isn't in v17 anymore.
So, I've adapted the current code.
The remaining question on my side is: where is it expected to change invoice amount_total in the code because in standard it will be 120 instead of 100.
Maybe test cases are wrong?

account_ecotax/views/account_move_view.xml

bealdav · 2024-06-19T13:14:02Z

some fix there akretion#7

epanisset · 2024-07-16T13:44:51Z

@mourad-ehm @bealdav
Hello,
Some news about this oca module? :)
Where do you stand?

Thanks

dreispt · 2024-07-24T13:56:55Z

Possible dumb question.
I recently configured Ecotaxes fro a customer, using OOB Taxes (nor for France, though).
I wonder what is the gap in the OOB Taxes calculation, so that custom code is needed.

Thanks!

gurneyalex · 2024-07-25T07:08:17Z

Possible dumb question. I recently configured Ecotaxes fro a customer, using OOB Taxes (nor for France, though). I wonder what is the gap in the OOB Taxes calculation, so that custom code is needed.

Certainly dumb question: what are "OOB Taxes"?

account_ecotax/models/account_ecotax_classification.py

account_ecotax/models/ecotax_line_product.py

gurneyalex · 2024-07-25T10:22:52Z

account_ecotax/models/ecotax_line_mixin.py

+    )
+    def _compute_ecotax(self):
+        for ecotaxline in self:
+            ecotax_cls = ecotaxline.classification_id


I recommend not abbreviating to "cls" : this makes developpers think of a Python class. When I see a variable called ecotax_cls, I think it refers to a Python class for Ecotax, not to a ecotax.classification record.

account_ecotax/models/ecotax_line_mixin.py

account_ecotax/models/product_template.py

gurneyalex

I'm globally worried by the move to account.tax to manage these. All my manual tests showed wrong results, at least from a FR fiscal perspective: wrong amount untaxed for the product (the "ecotax" should not be removed, only the VAT).

If I sell a product 100€ (HT) with 20% VAT and 1.12 écocontribution included to my customer. the amount untaxed on the invoice must be 100€, not 98.88€. There must be a label saying that in these 100€, there is 1.12€ of ecocontribution. And in my VAT report, the base amount that shows up must be 100€.

gurneyalex · 2024-07-30T08:38:02Z

account_ecotax/tests/test_ecotax.py

+                "company_id": cls.env.user.company_id.id,
+            }
+        )
+        # 2- Invoice tax with included price to avoid unwanted amounts in tests


We need a set of tests with taxes not included in price. The default tax in France in Odoo is a 20% G which is not included in price, switching this is not an option for FR users.

gurneyalex · 2024-07-30T08:43:46Z

account_ecotax/tests/test_ecotax.py

+        self._run_checks(
+            invoice,
+            {"amount_ecotax": 5.0 * new_qty, "amount_total": 100.0 * new_qty},
+            [{"ecotax_amount_unit": 5.0, "subtotal_ecotax": 5.0 * new_qty}],


I had failures on this when running the test manually through the UI: the subtotal_ecotax was staying the same.

gurneyalex · 2024-07-30T08:47:41Z

account_ecotax/models/account_tax.py

+    def onchange_is_ecotax(self):
+        if self.is_ecotax:
+            self.amount_type = "code"
+            self.include_base_amount = True


Do we need to set include_base_amount too?

Yes. It is a odoo method to compute imbricated taxes

gurneyalex · 2024-07-30T08:49:16Z

account_ecotax/models/account_move_line.py

+            ecotax_ids = line.tax_ids.filtered(lambda tax: tax.is_ecotax)
+
+            if line.display_type == "tax" or not ecotax_ids:
+                continue


set computed values to 0?

gurneyalex · 2024-07-30T08:51:08Z

account_ecotax/models/account_move_line.py

+    )
+    ecotax_amount_unit = fields.Float(
+        digits="Ecotax",
+        string="Ecotax Unit",


This name is confusing. Is it a unit price?

account_ecotax/models/account_move_line.py

gurneyalex · 2024-07-30T08:54:47Z

account_ecotax/models/account_move_line.py

+            if line.display_type == "product" and line.move_id.is_invoice(True):
+                amount_currency = line.price_unit * (1 - line.discount / 100)
+                handle_price_include = True
+                quantity = line.quantity


missing: handling of the UoM on the line

dreispt · 2024-07-30T10:47:07Z

Certainly dumb question: what are "OOB Taxes"?

"Out Of the Box" 😺

epanisset · 2024-07-30T12:44:17Z

Certainly dumb question: what are "OOB Taxes"?

"Out Of the Box" 😺

(Edited following further tests)

Ecotax (in France) is not a real "tax".
It's included in price, it has to be shown on the invoice (per line) + total at the bottom of the invoice.
Using only OBB tax is complicated, and incorrect in term of display.

How to manage the ecotax based on wieght with OBB taxes, with python code?

I am curious :)

bealdav · 2024-07-30T16:43:25Z

Don't forget that you can set your tax as include OR NOT in the Price with native odoo settings.
So you can adjust according to your client requirements.
Maybe I missed something. @gurneyalex

epanisset · 2024-07-31T09:42:45Z

Don't forget that you can set your tax as include OR NOT in the Price with native odoo settings. So you can adjust according to your client requirements. Maybe I missed something. @gurneyalex

@bealdav @gurneyalex After further testing,
With the "included tax", it's working well.
We started from the statement that the tax would take off from the price the ecotax, but it's not the case 👍

I still have issue when I buy multiple qty of a product. The ecotax is divided instead of multiplied.

I am supposed to get Ecotax = 30€ and Ecotax unit = 10€.

Careful also with the invoice PDF, we are supposed to identified easily the ecotax there, per line and in total. Not the case right now.

Some examples allowed:

gurneyalex · 2024-09-02T13:18:12Z

@mourad-ehm are you still working on this?

bealdav · 2024-09-02T16:29:30Z

@gurneyalex Mourad is on holidays.

You can join me tomorrow with my mail/phone.

Thanks

rvalyi · 2024-10-05T13:38:39Z

@bealdav @mourad-ehm @alexis-via @gurneyalex @epanisset In this commit 7adccf2 I made a few fixes in the tests where recent field renames were not properly applied

I left the commit as e separate commit so you can easily see what I did, but before merging it will have to be squashed into the "Apply suggestions from code review " commit. BTW, see the OCA guidelines for the commit messages here:
https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#71commit-message (this is important later on when people do administrative tasks such as forward and back ports).

Despite my fixes there are still failing tests with wrong tax values...

gurneyalex · 2024-10-14T08:45:00Z

I tested again this morning on runboat, the issue with the tax not being multiplied by the invoiced qty is still there. I expect the ecotax to be 15.40 in this case.

bealdav · 2024-10-14T08:53:18Z

Please check formulae in python tax computed, adjust it.
Yes we have to make it clear

bealdav · 2024-10-14T08:54:50Z

cc @mourad-ehm please could you plan to make final commits next week please. It becomes necessary now. Thanks

gurneyalex · 2024-10-14T08:59:08Z

Other failing test: Set the ecotax classification on the product to weight based

I set the weight of the product to 40kg so the ecotax amount is 1.2

An invoice for the product shows ecotax amount of 0

But displaying the details show the amount of 1.2

gurneyalex · 2024-10-14T09:04:54Z

account_ecotax/models/account_tax.py

+# result = product.weight_based_ecotax or 0.0
+result = product.fixed_ecotax or 0.0


Suggested change

# result = product.weight_based_ecotax or 0.0

result = product.fixed_ecotax or 0.0

if product.weight_based_ecotax:

result = product.weight_based_ecotax or 0.0

else:

result = product.fixed_ecotax or 0.0

gurneyalex · 2024-10-14T09:12:02Z

account_ecotax/models/account_move_line.py

+            for tax in compute_all_currency["taxes"]:
+                subtotal_ecotax += tax["amount"]
+
+            unit = subtotal_ecotax / quantity if quantity else subtotal_ecotax


Suggested change

unit = subtotal_ecotax / quantity if quantity else subtotal_ecotax

unit = subtotal_ecotax * quantity if quantity else subtotal_ecotax

see https://github.com/OCA/l10n-france/blob/04243404949a9dc13bd45a8e8c4675e5332493b1/l10n_fr_ecotaxe/models/account_move_line.py#L27C21-L27C62

Unit represent the ecotax amount for 1 product (for reporting purpose). The multiplication by the quantity is set in python code of taxes (see the readMe file).

mourad-ehm · 2024-10-29T11:22:21Z

/rebase

Co-authored-by: Maksym Yankin <[email protected]>

…test

Co-authored-by: Alexandre Fayolle <[email protected]>

mourad-ehm · 2024-10-29T15:08:49Z

Hi @gurneyalex I improved RedMe file to show how to configure ecotaxes and so fix the qty issue.

bealdav mentioned this pull request May 27, 2024

Generalization/Internationalization of l10n_fr_ecotaxe OCA/l10n-france#483

Closed

bealdav mentioned this pull request Jun 5, 2024

16.0 mig l10n fr ecotaxe OCA/l10n-france#448

Merged

bealdav force-pushed the 16-Add-account_ecotax branch 2 times, most recently from 4b800eb to 4875e19 Compare June 7, 2024 11:10

yankinmax reviewed Jun 10, 2024

View reviewed changes

account_ecotax/__manifest__.py Outdated Show resolved Hide resolved

yankinmax reviewed Jun 10, 2024

View reviewed changes

account_ecotax/models/account_ecotax_classification.py Outdated Show resolved Hide resolved

yankinmax reviewed Jun 10, 2024

View reviewed changes

account_ecotax/models/account_ecotax_classification.py Outdated Show resolved Hide resolved

yankinmax reviewed Jun 10, 2024

View reviewed changes

account_ecotax/models/account_move_line.py Outdated Show resolved Hide resolved

yankinmax approved these changes Jun 10, 2024

View reviewed changes

yankinmax reviewed Jun 10, 2024

View reviewed changes

account_ecotax/models/account_move_line.py Outdated Show resolved Hide resolved

yankinmax reviewed Jun 10, 2024

View reviewed changes

account_ecotax/tests/test_ecotax.py Show resolved Hide resolved

This was referenced Jun 10, 2024

[17.0][MIG] account_ecotax: Migration to 17.0 #433

Open

[17.0][MIG] l10n_fr_ecotaxe_sale: Migration to 17.0 OCA/l10n-france#551

Closed

bealdav reviewed Jun 19, 2024

View reviewed changes

yankinmax reviewed Jun 19, 2024

View reviewed changes

bealdav reviewed Jun 19, 2024

View reviewed changes

account_ecotax/views/account_move_view.xml Outdated Show resolved Hide resolved

gurneyalex reviewed Jul 25, 2024

View reviewed changes

gurneyalex reviewed Jul 30, 2024

View reviewed changes

gurneyalex mentioned this pull request Sep 2, 2024

WIP [16.0] add account_ecotax_sale module #429

Open

rvalyi force-pushed the 16-Add-account_ecotax branch from 2c6b35d to 7adccf2 Compare October 5, 2024 13:32

gurneyalex reviewed Oct 14, 2024

View reviewed changes

mourad-ehm and others added 12 commits October 29, 2024 15:10

[16.0] add account_ecotax module

f4d842b

misc-fix2review

577fc44

Apply suggestions from code review

0cfeb6f

Co-authored-by: Maksym Yankin <[email protected]>

[16.0] FIX code indentation & pre-commit

d30c342

[16.0] FIX module version

0ce73c2

[16.0] FIX test & exclude account_product_fiscal_classification from …

faa8a0c

…test

[16.0] Ecotax lines is readonly & reorganize view fields

758dfc8

Update account_ecotax/models/account_ecotax_classification.py

ce8ad37

Co-authored-by: Alexandre Fayolle <[email protected]>

Apply suggestions from code review

563aad6

Co-authored-by: Alexandre Fayolle <[email protected]>

FIX typo & classification view

ed92a1c

[FIX] account_ecotax: test fixes

ec38154

IMP readme usage

fc67913

mourad-ehm force-pushed the 16-Add-account_ecotax branch from 7adccf2 to fc67913 Compare October 29, 2024 14:53

FIX tests

ff0cdc3

florian-dacosta mentioned this pull request Nov 18, 2024

[16][REF] account_ecotax split #454

Open

epanisset mentioned this pull request Nov 19, 2024

Split of the ecotax modules #453

Open

florian-dacosta force-pushed the 16-Add-account_ecotax branch from 40068e1 to ff0cdc3 Compare November 28, 2024 11:25

		# result = product.weight_based_ecotax or 0.0
		result = product.fixed_ecotax or 0.0

	unit = subtotal_ecotax / quantity if quantity else subtotal_ecotax
	unit = subtotal_ecotax * quantity if quantity else subtotal_ecotax

WIP [16.0] add account_ecotax module #428

Are you sure you want to change the base?

WIP [16.0] add account_ecotax module #428

Conversation

mourad-ehm commented May 23, 2024

yankinmax left a comment

Choose a reason for hiding this comment

bealdav Jun 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bealdav Jun 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yankinmax commented Jun 19, 2024 • edited Loading

bealdav commented Jun 19, 2024

epanisset commented Jul 16, 2024

dreispt commented Jul 24, 2024

gurneyalex commented Jul 25, 2024

Choose a reason for hiding this comment

gurneyalex left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dreispt commented Jul 30, 2024

epanisset commented Jul 30, 2024 • edited Loading

bealdav commented Jul 30, 2024 • edited Loading

epanisset commented Jul 31, 2024

gurneyalex commented Sep 2, 2024

bealdav commented Sep 2, 2024

rvalyi commented Oct 5, 2024 • edited Loading

gurneyalex commented Oct 14, 2024 • edited Loading

bealdav commented Oct 14, 2024

bealdav commented Oct 14, 2024

gurneyalex commented Oct 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mourad-ehm commented Oct 29, 2024

mourad-ehm commented Oct 29, 2024

bealdav Jun 19, 2024 •

edited

Loading

bealdav Jun 19, 2024 •

edited

Loading

yankinmax commented Jun 19, 2024 •

edited

Loading

epanisset commented Jul 30, 2024 •

edited

Loading

bealdav commented Jul 30, 2024 •

edited

Loading

rvalyi commented Oct 5, 2024 •

edited

Loading

gurneyalex commented Oct 14, 2024 •

edited

Loading